Migrate langfuse to v3 #2247
Migrate langfuse to v3 #2247
Conversation
github-actions
bot
added
integration:langfuse
type:documentation
|
@LastRemote would you please, at your convenience, try out this PR on your async setup we used to previously test #2207 It would be amazing to nail this transition to v3 without regressions and additional cleanups. 🙏 |
|
@LastRemote did you have a chance to run this branch on your async setup? 🙏 |
|
@vblagoje Not yet. We had some trouble self-hosting Langfuse v3 so I haven’t started. I am planning to do this at some point this week after my experiment with critic agents. |
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the Langfuse integration from v2 to v3, which is built on OpenTelemetry (OTel). The migration introduces an expected extra root span level due to OTel's requirement that trace attributes must be held by a span, unlike v2's direct trace creation capability.
Key changes:
- Updated Langfuse dependency from
>=2.9.0, <3.0.0to>=3.0.0, <4.0.0 - Refactored span creation to use OTel-based context managers instead of direct trace/span creation
- Updated tests to accommodate the new API and extra root span behavior
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pyproject.toml | Updates Langfuse dependency to v3 |
| tracer.py | Core migration from v2 to v3 API with context manager-based span creation |
| test_tracer.py | Updates unit tests with new mock structure for v3 API |
| test_tracing.py | Updates integration tests to handle extra root span and configurable host |
Comments suppressed due to low confidence (1)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -456,4 +479,4 @@ def get_trace_id(self) -> str: | |||
| Return the trace ID. | |||
| :return: The trace ID. | |||
| """ | |||
| return self._tracer.get_trace_id() | |||
| return self._tracer.get_current_observation_id() | |||
There was a problem hiding this comment.
The method get_trace_id() should return a trace ID, but it's calling get_current_observation_id() which returns an observation ID. This appears to be incorrect - it should likely call get_current_trace_id() instead.
| return self._tracer.get_current_observation_id() | |
| return self._tracer.get_current_trace_id() |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
I agree that there is a name mismatch here that we at least should explain or fix if the intention is to call get_current_trace_id
There was a problem hiding this comment.
Yes, I'll rename it - both methods have exactly the same semantics!
| @@ -119,7 +122,6 @@ def test_tracing_integration(llm_class, env_var, expected_trace, basic_pipeline) | |||
| ) | |||
| @pytest.mark.integration | |||
| def test_tracing_with_sub_pipelines(): | |||
|
|
|||
| @component | |||
There was a problem hiding this comment.
Remove the extra empty line before the @component decorator to maintain consistent spacing.
Copilot uses AI. Check for mistakes.
| mock_client.start_as_current_span.return_value = MockContextManager() | ||
| mock_client.start_as_current_observation.return_value = MockContextManager() | ||
| mock_client.get_current_trace_id.return_value = "mock_trace_id_123" |
There was a problem hiding this comment.
These three lines seem redundant given that in the line above we call mock_get_client, which sets
mock_client.start_as_current_span = Mock(return_value=MockContextManager())
mock_client.start_as_current_observation = Mock(return_value=MockContextManager())
mock_client.get_current_trace_id = Mock(return_value="mock_trace_id_123")It occurs a couple of times in the file, for example test_trace_generation and test_trace_generation_invalid_start_time. Just check all the places where we call mock_get_client()
| if trace_attrs: | ||
| # We need to get the actual span from the context manager | ||
| # For now, we'll skip this as the context manager needs to be entered | ||
| pass |
There was a problem hiding this comment.
This code block doesn't do anything. Could be removed? What does "For now" mean? :)
| @@ -456,4 +479,4 @@ def get_trace_id(self) -> str: | |||
| Return the trace ID. | |||
| :return: The trace ID. | |||
| """ | |||
| return self._tracer.get_trace_id() | |||
| return self._tracer.get_current_observation_id() | |||
There was a problem hiding this comment.
I agree that there is a name mismatch here that we at least should explain or fix if the intention is to call get_current_trace_id
|
My comments from our earlier conversation Content pages to check if there are any updates needed:
Do you think we could leave a note somewhere in the docs about how to use the API SDK version 2 with langfuse-haystack<=2.3.0 or langfuse-haystack<3.0.0? Any new features that users benefit from that we can highlight? Improved simplicity?
Shall we ask to get listed here? https://langfuse.com/changelog/2025-06-05-python-sdk-v3-generally-available |
I really don't think we are adding some amazing value by moving from 2.x to 3.x
I don't think there is some great benefit except that users already use some other feature of Langfuse i.e. prompt management, they don't have to bundle two versions of the same library in their deployments.
We can, I can ask Hassieb |
|
Have another look @julian-risch 🙏 |
Yes, let's hear from @LastRemote - he's using langfuse integration in all sorts of edge cases. If he says this PR doesn't break any of them I'd be very confident about going forward with this PR. |
@vblagoje lol true. I did lots of customizations around Agent and ChatMessage, and in particular I extended the base class That being said, the new context_manager approach of LangfuseSpan is a little harder to play around since I cannot do something like I switched to the public hosted Langfuse to test this out. I am not seeing session-id/user-id in Langfuse traces, though I found this information in the logs. Here are the full trace details: 
|
|
@LastRemote what about those async traces - do they still work? I don't think we have "official" support for session ids, it's still in this DIY phase as suggested by @sjrl in this comment? |
|
@vblagoje Asyncs are fine. But I thought session-id/user-id were working? There was code supporting back then when we introduced |
I know - I was confused myself as I could not find any code where values in |
You are right that these are not explicitly set anywhere in this repository. Those are variables to be set and managed by pipeline providers / service providers to log multi-round or multi-user applications. For example, I am currently deploying pipelines as services and I allow the API caller (they should be able to define what is a session/user in their intended use case) to include session-id/user-id/tags in the headers. Then I will populate |
Understood, and makes sense. And what you are saying is that they don't show up now? And they are showing up in the current version of langfuse-haystack , not this PR? |
Yes. They were working correctly until then. I am still able to find them in the span details (full log above), but it seems like they are not recognizable by Langfuse anymore. |
|
They work for me @LastRemote - I used the following itinerary agent snippet: token = None
try:
# Get user input interactively
user_input = questionary.text(
"What kind of itinerary would you like me to create for you? For example: A 4-day trip in the south of France."
).ask()
if not user_input:
print("No input provided. Exiting...")
return
# Create a hash of the user input to use as a session ID
session_id_hash = hashlib.md5(user_input.encode()).hexdigest()
token = set_langfuse_context(session_id=session_id_hash, user_id="vblagoje", version="1.0.0")
response = macro_itinerary_agent.run(
messages=[
ChatMessage.from_user(text=user_input)
]
)
if not use_streaming:
print(response["messages"][-1].text)
finally:
for tool in all_tools:
if (isinstance(tool, MCPToolset)):
tool.close()
reset_langfuse_context(token)
So to me it seems to work. The functions |
|
@vblagoje Interesting, let me double check my code. Have you observed similar span object like the one I shared above? |
|
No, I didn't - I'm using LangfuseConnector, the code is at https://github.com/vblagoje/itinerary-agent I'm getting the above trace by adding var context to that itinerary agent. That's all. |
|
I am so confused. I just realized I am also not seeing the root pipeline span (which might be the reason), although I should not have customized that part. Feel free to merge; I think this issue is much more likely to be on my end. I will share more updates when I understand the situation better. |
|
And may I ask where is the root trace/pipeline span being created? Would this be related to async process? |
|
Yes, the root level is no more as it used to be. To follow OTEL Langfuse changed it so we have additional root level now, see description of this PR |
|
Having confirmed that complex agent pipelines are properly traced, and async pipelines as well (🙏 @LastRemote ), along with a proof that users can still use context var (see above) I moved forward to integrate this PR. Release is https://pypi.org/project/langfuse-haystack/3.0.0/ |
Why
This PR migrates the Langfuse integration from Langfuse v2 to v3. Langfuse v3 is built on OpenTelemetry (OTel), which differs from v2's trace creation model. The migration introduces an expected extra root span level due to OTel's requirement that trace attributes must be held by a span, unlike v2's direct trace creation capability.
TLDR - you'll see an extra nested root span.
Proposed Changes
How did you test it?
Notes for the reviewer
The extra root span level is expected behavior in v3 due to OTel's architecture - it's not a bug but a fundamental difference from v2. Some tests needed to be updated to account for this.